Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the platform check and use the one from the current environment. #5758

Merged
merged 2 commits into from
Apr 3, 2024

Conversation

MrPetru
Copy link
Contributor

@MrPetru MrPetru commented Mar 28, 2024

Changes

  • Install : don't check inkscape capabilities if disableGraphics is True
  • IE install : build Gaffer on current platform

Checklist

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable.
  • I have tested my change(s) in the test suite, and added new test cases where necessary.
  • My code follows the Gaffer project's prevailing coding style and conventions.

MrPetru added 2 commits March 25, 2024 16:33
There is a way to disable the generation of some graphics, like for example
when Inkscape is not available. This is done by providing the INKSCAPE
argument to the build command and setting it to “disableGraphics”.

However this was still running Inkscape, and in our case it was failing.
Changes in this commit should avoid that.
We used to validate the current platform against a hard-coded list.
But now we decided to rely on the build environment to define and
use the correct platform.
@ivanimanishi ivanimanishi requested a review from johnhaddon March 28, 2024 23:29
Copy link
Member

@johnhaddon johnhaddon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with the caveat that I haven't checked the IE build changes.

@johnhaddon johnhaddon merged commit 4beed22 into GafferHQ:1.3_maintenance Apr 3, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants